Skip to content

gh #358 signalEOS() description update#359

Open
hari22yuva wants to merge 6 commits intodevelopfrom
feature/gh358_signalEOS_description_update
Open

gh #358 signalEOS() description update#359
hari22yuva wants to merge 6 commits intodevelopfrom
feature/gh358_signalEOS_description_update

Conversation

@hari22yuva
Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI review requested due to automatic review settings March 19, 2026 15:49
@github-project-automation github-project-automation bot moved this to Architecture Review Required in halif_aidl Mar 19, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates AIDL interface documentation for the audio/video decoder HALs to clarify post-signalEOS() behaviour and how EOS is indicated via FrameMetadata.endOfStream, aligning the contract discussed in gh #358 with the interface comments.

Changes:

  • Clarify decodeBuffer() return behaviour when signalEOS() has already been called.
  • Document that decodeBuffer() calls after signalEOS() (until flush/restart) must return false.
  • Refine FrameMetadata.endOfStream wording to describe EOS signalling on the final output callback.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
videodecoder/current/com/rdk/hal/videodecoder/IVideoDecoderController.aidl Updates decodeBuffer() / signalEOS() documentation to specify false after EOS until flush/restart.
videodecoder/current/com/rdk/hal/videodecoder/FrameMetadata.aidl Clarifies meaning/timing of endOfStream for video decoder output.
audiodecoder/current/com/rdk/hal/audiodecoder/IAudioDecoderController.aidl Mirrors the video decoder documentation updates for audio decoder EOS behaviour.
audiodecoder/current/com/rdk/hal/audiodecoder/FrameMetadata.aidl Clarifies meaning/timing of endOfStream for audio decoder output (plus whitespace clean-up).

@hari22yuva hari22yuva linked an issue Mar 19, 2026 that may be closed by this pull request
sensible changes

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 19, 2026 16:07
Ulrond
Ulrond previously approved these changes Mar 19, 2026
Copy link
Copy Markdown
Collaborator

@Ulrond Ulrond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works for me

@github-project-automation github-project-automation bot moved this from Architecture Review Required to Approved For Merge in halif_aidl Mar 19, 2026
@Ulrond Ulrond added documentation Improvements or additions to documentation component:audiodecoder SOC component: audiodecoder component:videodecoder SOC component: videodecoder labels Mar 19, 2026
@Ulrond Ulrond added this to the 0.14.0 milestone Mar 19, 2026
@Ulrond Ulrond self-assigned this Mar 19, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates AIDL documentation to clarify end-of-stream (EOS) semantics for the audio/video decoder controllers and their FrameMetadata.endOfStream output flag, aligning expected decodeBuffer() behaviour after signalEOS().

Changes:

  • Document that decodeBuffer() returns false if called after signalEOS() until the decoder is flushed or restarted.
  • Clarify that FrameMetadata.endOfStream is an output flag set on the final frame callback after EOS, once all queued frames are output.
  • Minor whitespace cleanup in audio FrameMetadata.aidl.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
videodecoder/current/com/rdk/hal/videodecoder/IVideoDecoderController.aidl Expands decodeBuffer()/signalEOS() documentation to specify post-EOS decodeBuffer() returns false.
videodecoder/current/com/rdk/hal/videodecoder/FrameMetadata.aidl Clarifies meaning/timing of endOfStream flag in decoder output metadata.
audiodecoder/current/com/rdk/hal/audiodecoder/IAudioDecoderController.aidl Mirrors video controller doc updates for post-EOS decodeBuffer() behaviour.
audiodecoder/current/com/rdk/hal/audiodecoder/FrameMetadata.aidl Clarifies meaning/timing of endOfStream flag and removes trailing whitespace.

You can also share your feedback on Copilot code review. Take the survey.

Copy link
Copy Markdown
Contributor

@bhanucbp bhanucbp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

address review comments

* End of stream indicator.
* End of stream flag for decoder output.
* Set to true on the final frame output callback after signalEOS(), once all queued frames have been output.
* This may correspond to an end of stream marker in the audio bitstream, when present.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

audio bitstreams will not have eos markers present

* End of stream indicator.
* End of stream flag for decoder output.
* Set to true on the final frame output callback after signalEOS(), once all queued frames have been output.
* This may correspond to an end of stream marker in the audio bitstream, when present.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is dummy onFrameOutput call with -1 pts and null framehandle after signalEOS(). This info missing, add this info to onFrameOutput

Copy link
Copy Markdown
Contributor Author

@hari22yuva hari22yuva Mar 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this SOC specific implementation ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is not implementation-specific. The callback behavior should be explicitly documented; otherwise, the caller will not be aware of the final dummy frame.

/**
* End of stream marker found in the video bitstream.
* End of stream flag for decoder output.
* Set to true on the final frame output callback after signalEOS(), once all queued frames have been output.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

its not on the final frame, there will be a dummy onFrameOutput with -1 pts and null frame handle after final frame delivered

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this implementation specific ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is not implementation-specific. The behavior of the callbacks needs to be clearly defined; otherwise, the caller may not know that a final dummy frame (with invalid PTS and null handle) is delivered after signalEOS().

@Ulrond Ulrond modified the milestones: 0.14.0, 0.15.0 Mar 23, 2026
Copilot AI review requested due to automatic review settings April 9, 2026 14:42
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the Audio/Video decoder AIDL interface documentation to clarify end-of-stream (EOS) behaviour, including how EOS affects subsequent buffer submission and how EOS is communicated in frame metadata.

Changes:

  • Documented that decodeBuffer() must return false after signalEOS() for both audio and video decoders.
  • Expanded FrameMetadata.endOfStream documentation to describe EOS signalling scenarios (including “EOS-only” callbacks).
  • Minor whitespace cleanup in audio FrameMetadata.aidl.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
videodecoder/current/com/rdk/hal/videodecoder/IVideoDecoderController.aidl Adds post-signalEOS() decodeBuffer() behaviour note.
videodecoder/current/com/rdk/hal/videodecoder/FrameMetadata.aidl Expands endOfStream semantics for video output callbacks.
audiodecoder/current/com/rdk/hal/audiodecoder/IAudioDecoderController.aidl Adds post-signalEOS() decodeBuffer() behaviour note.
audiodecoder/current/com/rdk/hal/audiodecoder/FrameMetadata.aidl Expands endOfStream semantics for audio output callbacks and removes trailing whitespace.

Copy link
Copy Markdown
Collaborator

@Ulrond Ulrond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's discuss alternative option, this seems clearer to me longer term

#380

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component:audiodecoder SOC component: audiodecoder component:videodecoder SOC component: videodecoder documentation Improvements or additions to documentation

Projects

Status: Approved For Merge

Development

Successfully merging this pull request may close these issues.

Bug: signalEOS() description update

5 participants